Skip to content

Preserve unsafe async reads with packed frame buffers - #106

Open
dgant wants to merge 1 commit into
JavaBWAPI:implement-memory-safefrom
dgant:codex/pr105-unsafe-async
Open

dgant wants to merge 1 commit into
JavaBWAPI:implement-memory-safefrom
dgant:codex/pr105-unsafe-async

Conversation

@dgant

@dgant dgant commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

When unsafe async moves a running bot from the live buffer to a packed frame, #105 can calculate an offset using one layout and read it using the other. Retained Event, Position, Command, and UnitCommand views also keep layout-dependent offsets across the handoff. This can return incorrect data or access invalid native memory.

Keep all generated offsets in the original BWAPI layout and translate them inside the selected WrappedBuffer at the primitive/string access. Publish ClientData.buffer through a volatile reference, remove the redundant dynamic overrides, and update the generator. Static-data routing, the four copy ranges, and unsafe async's start-before-copy scheduling stay unchanged. Packed slots remain 4,805,308 bytes instead of 33,017,048 bytes.

This PR is based on #105 (a1574049) and targets its implement-memory-safe branch.

Validation:

  • Both handoff regression tests fail on Reduce Async FrameBuffer memory usage #105 and pass with this fix.
  • All 52 tests pass on Java 8, with automatic retries disabled.
  • The new synchronization test holds the first frame copy until the bot starts, proving unsafe overlap is preserved; the game then completes across multiple buffer cycles.
  • All 10 focused AsyncClientData/WrappedBuffer/UnsafeTools tests pass on Java 27.
  • Generator output is byte-for-byte reproducible; existing file line endings are preserved.

}

private long addressAt(int offset) {
// Translate on the selected buffer: unsafe async can switch layouts during a read.

@JasperGeurtz JasperGeurtz Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do note that this approach requires quite a few branches, I'm not sure this is good for performance. @ Astra is there really no way to make the original approach work (fixing what was wrong), but making it really statically calculated offsets instead of these branches here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Actually Dan) Yeah static offsets would be better. Will get you that.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants